feat(platform)!: delete and erase lifecycle for keep-history documents by shumkov · Pull Request #4657 · dashpay/platform

shumkov · 2026-09-10T22:55:39Z

Stacked on #4652 (keep-history storage v2). Review that PR first: until #4652 merges, this PR's diff against v4.3-dev includes #4652's commits (the base points at v4.3-dev so the full CI workflow and bot reviews run). Only the commits after b1442a767d belong to this PR.

Issue being fixed or feature implemented

Document types with documentsKeepHistory: true store every revision, and until
now their documents could not be removed at any layer. The shared-editing ACL
work requires history on shared-editing types, so without a lifecycle every such
document would be permanent.

This adds one: a delete removes a keep-history document from ordinary reads and
leaves its revisions readable, and a separate, separately permitted erase purges
those revisions when they are no longer wanted. It supersedes the product rule of
#4218, which treated documentsKeepHistory and canBeDeleted as a contradiction
because the storage layer refused the delete. The storage layer no longer does.

What was done?

Contract grammar. A keep-history type may allow deletion. It may
additionally set the new canBeErased, which requires history and deletion, is
immutable across updates, and defaults to false. A keep-history type may not
carry a contested index: a contested resource is awarded outside transition
validation, at an id derived from the winner rather than from the contested
values, and that award can land on an id whose retained history already exists.

Four states. Active while the document is visible; Deleted once the current
pointer and its index references are gone and a lifecycle record naming the
deletion time stands in their place; Erasing once an authorized erasure has
begun; Absent when nothing is left and the id is free again. The record lives in
a per-type tree beside the history tree and exists exactly while a document is
deleted or erasing.

Delete never escalates. It has no code path that removes a revision, so a
second delete is a paid consensus error rather than a deeper removal. A document
that has been deleted keeps its id: creating over it is refused both in
transition validation and in the storage writer, which is what covers writers
that never pass through validation at all.

Erasure is authorized once. The owner commits the document to erasure; the
record that commitment leaves in state is then the evidence that destruction was
authorized, so any identity may submit the remaining chunks and pay for them. An
owner who loses their keys, their funds or their permission cannot strand a
half-erased document. One transition removes at most
max_document_revisions_erased_per_transition revisions, newest first, so a
partial erasure leaves the oldest content and a contiguous sequence behind; the
terminal chunk also drops the record and the now empty history subtree, which
GroveDB accepts only because the revision deletes are in the same batch.

Observability. getDocumentHistory response v1 reports the two new states
and the four times the record carries, derived identically in the fetch and the
verifier, and the proof verifier checks every field against the proof.

Clients. rs-sdk gains DocumentEraseTransitionBuilder, Sdk::document_erase
and a separately named Sdk::document_current_lifecycle; wasm-sdk gains
documentErase; js-evo-sdk gains documents.erase(). The erase's own proof
shows the document absent by id, which it already was before the erase ran, so
it is classified as affected state rather than proved execution. That is why the
SDK takes the affected-state wait: the strict wait refuses exactly this
classification, and would return an error for an erase that had executed. The
result is named for what it authenticates, and the lifecycle read is offered
under its own name.

Wire and action shape. There were two ways to carry a new transition
kind. The book's frozen-generation rule, read strictly, asks for a new batch
wire format and a new generation of every batch-level validator that could
see the kind. The way every earlier kind was added asks for none of that:
document transfer (#1826), NFT purchase and price update (#1829), the token
transitions (#2383) and the indexOnly delete (#4497) were each appended to
the shipped DocumentTransition / BatchedTransition enums and handled by
arms inside the existing basic structure, advanced structure, state and
transformer v0 generations, with the batch action staying V0. We chose the
second. The erase kind is appended to the shipped DocumentTransition and
its action to the shipped DocumentTransitionAction; batch wire formats 0
and 1 carry it, and there is no new batch format, no new batch action format
and no new batch-level generation. Admission is gated per kind, as #4497 did:
validate_basic_structure/v0 refuses an erase wherever the active protocol
version publishes no bounds for it (document_erase_state_transition is
None in every released serialization table and Some(0..=0) at protocol
15), so software that knows the kind agrees with software that cannot decode
it while protocol 14 is still active: the old node fails at decode, the new
node at the gate, both as an unpaid consensus error, and a block containing
one is rejected by both. The arms added to the shipped generations sit behind
that gate and are byte-identical for every historical input.

Accounting. Removing revisions credits whoever paid for each of them, in
balance updates applied after the transition's fee result is formed against
identities that had nothing to do with it. The chunk bound limits how many there
can be; the erase is charged for them, in both the estimate that admits it and
the fee it pays. The shared per-type lifecycle container is unflagged, so no
single deleter is charged for structure nobody refunds; the record inside it
carries the deleter's flags and is what an erase refunds.

Versioning. Everything rides protocol 15, which is unreleased. Protocol 14
ships with Platform 4.2, so every behaviour change of an existing path lives in
a new generation reached only from PLATFORM_V15 (contract parser generation
4 with meta-schema v4 and DocumentTypeV3, document type update validation
v2, Drive delete wrappers v1, insert generation 2, erase operations v0, delete
structure v2 and state v1, create state v3, erase structure and state v0). The
history query handler arrived with the protocol 15 storage layout in #4652, so
it reports the deleted and erasing states in place rather than through a
second generation. The batch wire bounds, the batch basic-structure
generation and the batch-level transformer, structure, state, nonce,
admission, conversion, prover and verifier generations are the same at
protocol 15 as at 14; a platform-version test pins that equality and pins the
released values literally. Released tables gain dormant None/0 slots only,
and protocols 12 through 14 replay unchanged: a keep-history delete still ends
in an internal error at 12 and 13 and in a paid rejection at 14, a keep-history
type that allows deletion is still refused by the protocol 14 parser, and a
batch carrying an erase is refused at protocol 14 as an unsupported version.

How Has This Been Tested?

cargo test -p drive --lib: 3,760 passed, 6 existing ignored;
--test query_tests_history: 5 passed.
cargo test -p drive-abci --lib -- batch::tests document_history lifecycle_contracts keep_history erase:
415 passed, 7 existing ignored (the erase suite alone: 18).
cargo test -p dpp --all-features --lib -- batch_transition keep_history erase document_type:
1,366 passed; cargo test -p platform-version: 26 passed;
cargo test -p drive-proof-verifier: 288 passed;
cargo test -p dash-sdk --features mocks -- history erase: 10 passed.
cargo test -p drive-abci --test strategy_tests -- document run_chain_v14_to_v15:
22 passed. The lifecycle over migrated storage is pinned at the drive layer
(should_keep_refusing_by_revision_reads_of_a_gapped_history_after_deletion
writes legacy-layout revisions at 14, migrates at 15, then deletes and
erases): a keep-history type registered at protocol 14 can never become
deletable, so the chain test cannot exercise it.
cargo clippy --workspace --all-targets --all-features -- -D warnings,
cargo fmt --all -- --check, cargo check -p drive --no-default-features --features verify,
and cargo check --target wasm32-unknown-unknown for wasm-sdk and
wasm-dpp2: clean.
Byte-identity audit against feat(drive)!: preserve composite document history across protocol activation #4652's head: the shipped batch modules differ
only by the appended-kind arms, the erase constructor and the resolver
method; every released table constant differs by dormant slots only.

New coverage worth calling out: erasure at one revision, exactly one chunk, one
more than a chunk and more than two chunks; the record written once and never
rewritten by a continuation; a continuation submitted by an identity that owns
nothing; two erases, a delete-plus-erase, and a delete-plus-create sharing one
block; refunds keyed under the epoch each byte was written in and credited to a
real balance; the admission estimate covering a full chunk whatever the document
retains; the erasing state proved and its every claimed field pinned against
what the verifier derives; identical state and fees with GroveDB's batch
consistency checking on and off; nonce bumps persisted on refusals; a populated
history subtree refusing to be dropped; a signed batch with an erase submitted
through process_raw_state_transitions at protocol 14 and refused as an
unsupported version before any state is read; the same batch decoding under
protocol 14 and being refused only by basic structure; the erase's bincode
discriminant pinned (an erase and a delete over one base differ in exactly one
byte, in both shipped batch formats); an erase demanding its type's key level
and committing no credits inside a batch action; the erase's execution proof
classified as affected state while a delete over the same proof stays
execution-proved; every delete entry point that carries only the block time
refusing a keep-history document instead of recording a fabricated block; and a
create with override_document over a deleted document's retained revisions
refused by the protocol 15 insert generation (the previous generation merged it
into the deleted history).

A functional spec, KeepHistoryDocument.spec.js, walks the whole story against a
running network. It is not run in CI-less local verification and needs
yarn start.

Breaking Changes

From protocol 15 a contract may combine documentsKeepHistory: true with
canBeDeleted: true, and may set canBeErased. Protocols 12 through 14 are
unaffected.
DocumentTransition gains Erase and DocumentTransitionAction gains
EraseAction, both appended; code that matched either exhaustively needs an
arm. DocumentTransitionActionType gains Erase, appended. DocumentType
gains a V3 variant. Batch wire formats are unchanged.
getDocumentHistory's lifecycle state gains DELETED and ERASING and four
times; a client that matched exhaustively on the two previous states needs
updating.
DocumentOperationType gains DeleteDocumentWithLifecycle (with a
deleter_id) and EraseDocument; the Drive delete family gains
_with_lifecycle entry points that take the block the delete belongs to, and
the entry points that carry only the block time refuse a keep-history
document from protocol 15.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

Notes for reviewers

The author of fix(platform): reject contradictory keep-history document deletes #4218 should know its product rule is superseded here.
Two release items carry over from PR 1 and are not addressed by this PR: the
Class C activation-migration rehearsal on authentic state, and a benchmark of
a maximum legal block of worst-case erases on validator hardware to confirm
the chunk bound of 100.
The wire and action shape is a deliberate choice against the strict reading
of the book's frozen-generation rule and in favour of the way every earlier
kind was added (see "Wire and action shape"). An earlier revision of this
branch took the strict reading, with batch wire format 2, batch action
format 1 and a generation of every batch-level validator behind them; that
was replaced by a5a3d20450. PastaClaw's blocking finding on the current
head asks for the strict reading back; it is declined for this reason.
Two low-level deletes of one type, combined into a single batch before either
is applied and before the type has a lifecycle container, both emit that
container's insert, because each document operation is converted without sight
of its siblings. GroveDB refuses the batch rather than committing half of it,
and a test pins that. Consensus cannot reach the shape: a batch state
transition carries exactly one document transition at every protocol version.
GroveDB's query surface has no key-only result shape over a range, so the
erase's revision enumeration reads bodies it discards. The estimate charges for
that honestly rather than claiming a key-only read.

🤖 Generated with Claude Code

coderabbitai · 2026-09-10T22:55:45Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 557e38cc-2af1-46d4-9468-3bba872d1d5b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

thepastaclaw · 2026-09-10T22:58:23Z

⛔ Final review complete — 1 blocking finding(s) (commit b3e8fe2) · triage: critical · Phase 2 only (queue backlog)

codecov · 2026-09-11T01:04:08Z

Codecov Report

❌ Patch coverage is 78.17564% with 835 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.52%. Comparing base (b1442a7) to head (1e8c9d3).

Files with missing lines	Patch %	Lines
...state_transition_was_executed_with_proof/v0/mod.rs	58.79%	75 Missing ⚠️
...t_type/class_methods/try_from_schema/common/mod.rs	55.04%	49 Missing ⚠️
...tion/validation/validate_basic_structure/v0/mod.rs	58.26%	48 Missing ⚠️
...s/rs-dpp/src/data_contract/document_type/v3/mod.rs	79.45%	38 Missing ⚠️
...kages/rs-dpp/src/validation/meta_validators/mod.rs	2.56%	38 Missing ⚠️
...ete/delete_document_for_contract_operations/mod.rs	69.84%	38 Missing ⚠️
packages/rs-dpp/src/state_transition/mod.rs	57.50%	34 Missing ⚠️
packages/rs-drive/src/drive/document/delete/mod.rs	82.06%	33 Missing ⚠️
...ument_type/class_methods/try_from_schema/v4/mod.rs	74.60%	32 Missing ⚠️
...ent_for_contract_with_named_type_operations/mod.rs	57.35%	29 Missing ⚠️
... and 59 more

Additional details and impacted files

@@                     Coverage Diff                     @@
##           keep-history-storage-v2    #4657      +/-   ##
===========================================================
- Coverage                    75.60%   73.52%   -2.09%     
===========================================================
  Files                         2849     2888      +39     
  Lines                       421354   438776   +17422     
===========================================================
+ Hits                        318567   322604    +4037     
- Misses                      102787   116172   +13385

Components	Coverage Δ
dpp	`70.55% <65.77%> (-2.16%)`	⬇️
drive	`76.00% <82.65%> (-2.53%)`	⬇️
drive-abci	`72.51% <79.10%> (-1.73%)`	⬇️
sdk	`∅ <ø> (∅)`
dapi-client	`∅ <ø> (∅)`
platform-version	`∅ <ø> (∅)`
platform-value	`85.94% <ø> (+0.52%)`	⬆️
platform-wallet	`∅ <ø> (∅)`
drive-proof-verifier	`31.20% <ø> (-0.08%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

thepastaclaw

Final validation — Phase 2 only (queue backlog)

The lifecycle and erasure implementation is generally well covered, but two correctness issues remain. The WASM/JavaScript API cannot represent the documented continuation case where an identity other than the document owner submits a later erase chunk, and the history proof verifier accepts incomplete metadata proofs that can be interpreted as false lifecycle states.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This is a large, intricate cross-cutting change that directly modifies consensus validation and state-transition serialization, funds accounting/refunds, peer-facing protobuf/query deserialization, and storage migration/lifecycle deletion behavior.
Phase 1 reviewers: not run (skipped for throughput: 15 PRs queued, above the 10 limit)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/wasm-sdk/src/state_transitions/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/document.rs:597-625: Expose the erase submitter separately from the document owner
  The WASM wrapper derives `owner_id` from the target document (`doc_inner.owner_id()` or the plain object's documented `ownerId`) and passes it as the final argument to `DocumentEraseTransitionBuilder::new`. On the Rust side, that field is the identity submitting and paying for the erase: it is used to fetch the contract nonce and to construct the outer batch transition. Therefore a continuation signed by an unrelated identity is built as though it were submitted by the original document owner, so the signer and transition owner do not match and validation rejects the continuation. Add a separately named submitter or payer identity to `DocumentEraseOptions`, use it for the builder and nonce lookup, and retain the document owner only as target metadata where needed. Defaulting the submitter to the document owner can preserve the first-owner call.

In `packages/rs-drive/src/drive/document/history/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/history/mod.rs:569-575: Require absence proofs for lifecycle metadata lookups
  `metadata_query` performs exact-key lookups for the current document pointer, the lifecycle record, and the per-document history tree, but verification sets `absence_proofs_for_non_existing_searched_keys` to `false`. With that option, GroveDB may return no row for a requested exact key without proving that the key is absent. `query.lifecycle()` then treats omitted metadata as absent and can derive `ABSENT`, `DELETED`, or another incorrect lifecycle state even when the authenticated root contains the omitted pointer or lifecycle record. A compromised DAPI server can consequently make clients accept a false lifecycle result. Verify this metadata query with absence proofs enabled and give the merged query a finite limit sufficient for all three exact terminal-key lookups.

thepastaclaw

Final validation — Phase 2 only (queue backlog)

The lifecycle implementation fixes both previously reported issues, but the reviewed range still contains four in-scope defects. Protocol-14 activation can reject valid legacy contracts, revision-based history queries can return authenticated false-empty results after deleting gapped histories, migration work is unbounded during activation, and SDK mocks lose lifecycle states and metadata. These issues require changes before merge.

🔴 3 blocking | 🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This large, intricate diff changes consensus validation and state-transition serialization, peer-facing gRPC/proof deserialization, persistent document storage/migration behavior, and accounting/refunds for deleting and erasing document revisions.
Phase 1 reviewers: not run (skipped for throughput: 15 PRs queued, above the 10 limit)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs:320: Preserve deserialization of valid legacy contested-history contracts
  `reject_contested_keep_history` is called unconditionally, including when `full_validation` is false. Protocol 13 permits a document type with retained history and a contested index, but protocol-14 activation loads every stored contract using the new parser with `full_validation = false`. A valid legacy contract with this combination therefore fails to deserialize while the migration inventories contracts, causing the activation hook to abort before the migration can run. Keep the registration-time restriction separate from legacy-state deserialization, or provide an explicit migration-compatible parsing path, and cover registration under protocol 13 followed by protocol-14 activation.

In `packages/rs-drive/src/drive/document/history/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/history/mod.rs:353-360: Keep revision-gap detection after a history document is deleted
  Revision and `StartAtRevision` queries use an offset of `revision - 1`, but the gap check is only performed while `active` is true. A migrated history can legitimately retain revisions such as `[1, 3]`; while active, querying revision 3 is rejected because the latest revision differs from the retained count. Once deletion removes the current pointer, `active` becomes false, the check is skipped, and querying revision 3 applies offset 2 to the two retained entries and returns an empty page. The empty result contains no entries for `decode_entries` to inspect, so both the fetch path and proof verifier accept a false absence. Retain authenticated metadata sufficient to establish contiguity after deletion, or reject revision-position queries when contiguity cannot be proven.

In `packages/rs-drive/src/drive/document/migration/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/migration/mod.rs:471-493: Bound and paginate legacy history migration work
  `history_migration_entries` constructs a `Query` with `insert_all()` and materializes the complete history subtree into a vector. The caller retains those entries while constructing migration copies and applies the work in the single protocol-activation transaction. Pre-protocol-14 keep-history documents have no bound on the number of retained revisions, so a contract can accumulate a sufficiently large paid history before activation. During activation every validator must read, deserialize, duplicate, and rewrite that entire history synchronously; a large legal history can exhaust validator memory or exceed activation processing limits and prevent the upgrade from completing. Migration needs bounded resumable work, or a pre-activation bound that is enforced before such histories can accumulate.

In `packages/rs-sdk/src/mock/requests.rs`:
- [SUGGESTION] packages/rs-sdk/src/mock/requests.rs:273-306: Preserve all lifecycle states and timestamps in SDK mocks
  The mock codec serializes lifecycle state as only `state == Active` and stores only `remaining_revisions`; it omits all four lifecycle timestamps. Deserialization consequently maps both `Deleted` and `Erasing` to `Absent` and resets lifecycle metadata to defaults. `expect_fetch` passes responses through this codec, so mocked callers cannot faithfully exercise deleted or erasing document behavior. The same lossy representation affects nested `DocumentHistoryProofInfo` data. Serialize the complete lifecycle state and all lifecycle fields, and add round-trip coverage for active, deleted, erasing, and absent results with non-default metadata.

In `packages/rs-drive/src/drive/document/lifecycle/tests.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/lifecycle/tests.rs:1568-1573: Exercise proof verification when testing tampered lifecycle claims
  The tampered lifecycle value is created after verification and only compared with the original using `assert_ne!`; it is never passed back through the proof verifier. This tests value inequality rather than rejection of a forged lifecycle claim, and would still pass if the verifier's new lifecycle-field checks were removed. Extend the response-level verifier harness with deleted and erasing results, mutate each lifecycle field independently while leaving the proof unchanged, and assert that `DocumentHistory::maybe_from_proof` rejects every tampered response.

thepastaclaw

Final validation — Phase 2 only (queue backlog)

The lifecycle changes are coherent, and the previously reported proof, mock, revision-gap, parser, and WASM submitter issues are fixed at the current head. One in-scope API correctness issue remains: the new WASM history query accepts JavaScript Numbers for exact u64 selectors, allowing values above 2^53−1 to be silently rounded before Rust parses them.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This is a large, intricate diff that changes consensus validation and peer-facing state-transition deserialization, moves funds through lifecycle erasure refunds, and introduces storage migrations affecting document history.
Phase 1 reviewers: not run (skipped for throughput: 14 PRs queued, above the 10 limit)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/wasm-sdk/src/queries/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/queries/document.rs:178-183: Reject unsafe JavaScript Numbers for exact history selectors
  The public TypeScript API accepts `number` for `startAtMs`, both fields of `startAfter`, `startAtRevision`, and `revision`, while the Rust boundary deserializes these values as `u64` at lines 257–269. JavaScript has already rounded any Number greater than `Number.MAX_SAFE_INTEGER` before Rust receives it; for example, `9007199254740993` becomes `9007199254740992`. The SDK can therefore query and verify a different timestamp or revision than the caller requested, without reporting an error. Require `bigint` for these exact selectors, or retain Number compatibility only after rejecting unsafe values at runtime.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Bound and paginate legacy history migration work — The migration helper still uses an unrestricted insert_all() query and materializes complete subtrees in memory, so the resource-exhaustion concern remains technically valid. However, the migration implementation was introduced by the stacked PR before this PR's stated boundary at commit d4f6776fd, and the PR explicitly limits this PR to commits after that boundary. This should be tracked with the stacked keep-history migration work rather than blocking this lifecycle PR.
- Follow-up: Track bounded or resumable activation migration separately in the keep-history storage migration work.

thepastaclaw

Final validation — Phase 2 only (queue backlog)

Revalidated all eight prior findings at 9da4990: seven are fixed, and activation-migration capacity remains deferred work belonging to the stacked storage PR. Two client-side bugs are confirmed; the contested-ID failure already exists in the declared stacked base and is retained only as a separate high-value follow-up. Verification was source-based; no tests were run in this verifier pass.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This large, intricate change directly modifies consensus authorization in packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_erase_transition_action/state_v0/mod.rs and destructive, refund-bearing storage operations in packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs, requiring scr
Phase 1 reviewers: not run (skipped for throughput: 18 PRs queued, above the 10 limit)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/wasm-sdk/src/state_transitions/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/document.rs:645-649: Forward erase signing settings from JavaScript to the builder
  The wrapper decodes PutSettings, including userFeeIncrease and stateTransitionCreationOptions, but forwards them only through with_settings. DocumentEraseTransitionBuilder::with_settings stores broadcasting settings without populating the separate fields that sign reads. Consequently, documents.erase({ ..., settings: { userFeeIncrease: 250 } }) signs with a zero fee increase, and supplied creation options are also ignored. Broadcasting cannot repair an already-signed transition. Forward both signing settings to their builder methods and add coverage asserting that non-default JavaScript settings reach transition construction.

In `packages/rs-sdk/src/platform/documents/transitions/erase.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/documents/transitions/erase.rs:127-134: Reject unsupported erase versions before reserving a nonce
  With an SDK pinned to protocol 12 or 13, a valid contract/type and successful nonce fetch reach this call with bump_first = true, advancing the cached contract nonce. Only afterward does DocumentEraseTransition::from_document reject the unavailable erase version, and the error returns without broadcasting or undoing the increment. Repeated rejected calls can advance the cache beyond the protocol's 24-revision window and cause subsequent valid document transitions to fail. Refreshing does not lower the reserved value because NonceCache::get_or_fetch_nonce preserves max(cached, platform). Validate erase availability and deterministic creation-version constraints before reserving the nonce, and add a regression asserting that unsupported construction leaves the cache unchanged.

Out-of-scope follow-up suggestions (2)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Track activation inventory limits and resumable migration separately — The migration still materializes complete history subtrees and type-wide index inventories in one activation transaction. However, packages/rs-drive/src/drive/document/migration/mod.rs is unchanged between the declared stacked base d4f6776 and this head, and the PR explicitly separates storage-migration release work from this lifecycle change. The reported testnet rehearsal measures one inventory rather than enforcing a bound on subsequent growth.
- Follow-up: Track activation-capacity acceptance with PR #4652 or release preparation: refresh the inventory and resource rehearsal before activation, define acceptance thresholds, and use bounded or resumable processing if those thresholds are exceeded.
Handle pending-contest document-ID collisions in the storage-v2 work — The contested-ID collision is a concrete storage-v2 concern, but not a newly introduced failure in this lifecycle PR. In stacked base d4f6776, add_document_to_primary_storage_v1 already calls batch_insert_if_not_exists for the current pointer and returns CorruptedDocumentAlreadyExists when it exists. Thus an ordinary document occupying a pending contender's ID already makes award_document_to_winner fail with override_document = false; the newly added history-tree guard merely returns the same class of error earlier. The comparison with the protocol-13 writer misses this existing protocol-14 behavior in PR #4652.
- Follow-up: Track this with PR #4652: reproduce a legacy conditional-contest contract with a pending contender and an ordinary document sharing its ID, then cover protocol-14 award processing and safe handling of collisions already present before activation.

thepastaclaw

Preliminary review — Phase 1 blocker gate

Two prior findings remain valid at the exact head. The lifecycle tamper test still checks only in-memory inequality rather than verifier rejection, and legacy history migration still loads an entire per-document subtree into memory in one activation transaction. The other eight prior findings are fixed by the current code.

Validated blockers were found by the Phase-1 review and confirmed by a fresh verifier. Phase 2 is deferred until a fresh same-head revalidation clears the blocker gate.

🔴 1 blocking

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-gate-verifier, role: verifier)

Triage: critical by gpt-6-astra (effort low) — This is a large, intricate cross-cutting change that directly modifies consensus state-transition validation, peer-facing protobuf deserialization, funds-refunding accounting, cryptographic proof verification, and persistent document storage/migration behavior.
Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 11% left, 5h 100% left), glm-5.3-flash (zai below 15% reserve: 5h 100% left, weekly 13% left)
Fresh verifier: gpt-6-astra — verifier; agent astra-gate-verifier
Phase 2 reviewers: not run (deferred by blocker gate)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/drive/document/lifecycle/tests.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/lifecycle/tests.rs:1559-1573: Exercise proof verification when testing tampered lifecycle claims
  (existing thread: https://github.com/dashpay/platform/pull/4657#discussion_r3995256902)
  The test mutates a clone of `verified.lifecycle` and asserts only that the clone differs from the verified value. It never serializes the altered lifecycle into a response and passes that response, with the original proofs unchanged, through `DocumentHistory::maybe_from_proof` or the document-history verifier. Consequently, the test would remain green if the verifier stopped checking the claimed lifecycle fields. Add response-level coverage for deleted and erasing pages that mutates each lifecycle field independently and asserts verifier rejection.

In `packages/rs-drive/src/drive/document/migration/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/migration/mod.rs:471-493: Bound and paginate legacy history migration work
  (existing thread: https://github.com/dashpay/platform/pull/4657#discussion_r3995256896)
  `history_migration_entries` constructs an unrestricted `Query` with `insert_all()` and materializes the complete history subtree into a `Vec` via `to_key_elements()`. The caller then retains these entries while constructing migration copies and applying the work in the single protocol-activation transaction. Legacy pre-protocol-14 keep-history documents have no retained-revision bound, so a single legally accumulated history can force every validator to read, deserialize, duplicate, and rewrite an unbounded amount of data synchronously. A testnet snapshot and timing measurement do not bound future mainnet state or growth before activation; sufficiently large state can exhaust memory or exceed activation limits and prevent the upgrade. Bound the migration with resumable pagination/streaming, or enforce a pre-activation limit before such histories can accumulate.

Protocol version 14 gains the slots the delete and erase lifecycle of keep-history documents needs: the erase transition's wire bounds, its structure and state validation, the lifecycle read every stateful check goes through, the keep-history branch of the document delete, the erase storage operation and its estimation, and the bound on how many retained revisions one erase transition may remove. Every new field is dormant in the released tables and in the hand-written mock, so no released protocol version changes behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Generation 3 of the document type parser no longer refuses documentsKeepHistory together with canBeDeleted. A delete on such a type removes the document from ordinary reads and leaves its retained revisions readable, so the two settings no longer contradict each other. It also admits canBeErased, which additionally allows a deleted document's revisions to be purged. The flag defaults to false, requires both documentsKeepHistory and canBeDeleted, and is immutable across contract updates: narrowing it once an erasure had begun would strand a partially erased document forever, and widening it would hand an irreversible operation to a type registered without it. The existing canBeDeleted narrowing stays, except on an erasable type, which could never reach a state erase acts on again. A keep-history type may not carry a contested index: a contested resource is awarded outside transition validation, at an id derived from the winner rather than from the contested values, and that award can land on an id whose retained history already exists. BREAKING CHANGE: at protocol version 14 a contract may combine documentsKeepHistory with canBeDeleted, which earlier drafts of that protocol version rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

DocumentTransition::Erase is appended after IndexOnlyDelete so every existing variant keeps its bincode discriminant, and carries only the base transition: which chunk it removes, and whether it starts or continues an erasure, are read from the document's committed lifecycle rather than signed by the submitter. Its JSON action is "erase" and it maps to a new DocumentTransitionActionType. The kind is gated on document_erase_state_transition, which is None before protocol version 14, so a node running an older protocol version refuses it with UnsupportedVersionError and agrees with software that cannot decode it at all. A result proof for an erase shows the document absent by id, which it already was before the transition ran, so the proof classifier reports the outcome as affected state rather than proved execution. A subscription filter never matches an erase: it carries no document values and acts on a document no document query can see. BREAKING CHANGE: DocumentTransition and DocumentTransitionActionType each gain a variant, so exhaustive matches over them must be extended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Deleting a keep-history document now removes its current pointer and the index references that lead to it, leaves every retained revision where it is, and writes a lifecycle record naming the block time it was deleted at. The record lives in a per-type tree at a reserved key next to the history tree, is created on demand so that a type whose documents are never deleted never pays for one, and carries the deleter's flags so its bytes come back to them. Non-keep-history deletes are delegated to the previous implementation unchanged. Erasing removes a bounded chunk of an already deleted document's revisions, newest first, so a partial erasure leaves the oldest content and a contiguous sequence behind. The enumeration asks for one revision more than a chunk may remove, so the operation knows before emitting anything whether it is the last: a terminal chunk also removes the record and the now empty history subtree, which GroveDB accepts only because the revision deletes are already in the same batch, while a non-terminal first chunk instead overwrites the record with the erasure it authorizes. The two never coincide, so one batch never carries two operations on the record's key. A document whose revisions are still retained cannot be created again: the storage writer refuses an insert whose history subtree already exists, which covers writers that never pass through transition validation. A dry run skips the probe and pays for it as a fixed cost so estimation and execution stay aligned. History v1 gains the deleted and erasing states, derived identically in the fetch and the verifier from the same authenticated record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The erase transition action mirrors the delete one: only a base, no token cost of its own, and a nonce bump alongside the storage operation it converts into. It is appended to the document action enum and to the action type mapping. The parser tests are rewritten around the rule generation 3 now applies: a keep-history type may allow deletion, may additionally allow erasure, may not carry a contested index, and may withdraw deletion only while it is not erasable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…erasing Deleting a keep-history document now consults its lifecycle rather than the ordinary by-id read, which cannot see a document that has already been deleted. A second delete is therefore a paid consensus error rather than a delete of something that never existed, and no delete has any path to removing a revision. Erasing is authorized once. Its structure validation checks what the contract alone decides: the type retains revisions, allows them to be purged, carries no contested index, and offers no token payment, which erase has no use for since the deletion it follows was already charged. Its state validation then reads the document's lifecycle: a current document must be deleted first, a merely deleted one may be committed to erasure only by its owner, and an erasure already committed may be continued by any identity, because the record left in state is itself the evidence that destruction was authorized. An owner who loses their keys, their funds or their permission can no longer strand a half-erased document. Creating over a deleted document's id is refused: the id stays reserved while its revisions are retained, and becomes free again once an erase has removed the last one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…query The history response's lifecycle block gains the two new states and the four times the lifecycle record carries. The proof verifier checks every one of them against the proof, so a node cannot claim a deletion time or an erasure that its own state does not authenticate. The batch action enums in both WebAssembly bindings gain the erase kind, keeping the two hand-written discriminant tables in agreement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tion Append `DocumentTransition::Erase` and `DocumentTransitionAction::EraseAction` to the shipped enums, the way the indexOnly delete kind was added, instead of introducing batch wire format 2, batch action format 1 and a generation of every batch-level validator, transformer, converter, prover and verifier that could read them. Admission is gated per kind: `validate_basic_structure/v0` refuses an erase wherever the active protocol version publishes no bounds for it, so software that decodes the kind agrees with software that cannot while protocol version 14 is still active. The arms added to the shipped transformer, structure, state, converter and verifier generations are unreachable below protocol version 15 and byte-identical for every historical input; data triggers are matched by action type, so an erase reaches none. Version tables: `STATE_TRANSITION_SERIALIZATION_VERSIONS_V4` keeps only the erase bounds (batch wire bounds stay `0..=1`, default 1); `STATE_TRANSITION_VERSIONS_V4` is removed and protocol 15 keeps the shipped batch structure generation; the batch conversion, prover, verifier, transformer, advanced structure, state, nonce and admission slots at protocol 15 are the same as at protocol 14; the format-aware document fetch slot is gone from every table. Released constants differ by dormant `None`/`0` slots only. Deleted: the `DocumentTransitionV1`/`BatchedTransitionV1` shells, `BatchTransitionV2`, the second accessor view, batch basic structure v1, `BatchTransitionActionV1`, `StateTransitionAction::BatchActionV1`, converter `batch_transition_v1`, prover v1, verifier v1, abci transformer v1, state v2, state validation v1, advanced structure v1, nonce v1 and is-allowed v1. Clients build a format 1 batch: the rs-sdk erase builder accepts batch formats 0 and 1, wasm-dpp2 wraps the shipped shells and keeps the `erase` action type, wallet-ffi and wasm-sdk read the shipped accessors. Tests: the boundary pin `validate_base_structure_v0_gates_erase_by_protocol_version` refuses a format 1 batch with an erase at protocol 14 with `UnsupportedVersionError` and admits it at the latest version; a platform-version pin holds every batch-level slot at protocol 15 equal to protocol 14; the verifier classification test and the batch action key-level test are ported onto the shipped types. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

github-actions · 2026-09-19T16:43:27Z

PR Hygiene

State: waiting-bots · commit 1e8c9d3fa2c5dd9bef0c16b5240e46b4573eee53

Bot changes request remains outstanding
Proceeded without coderabbitai: no review within the configured window

Self-review is an author attestation that you have read the diff:
/self-reviewed — covers everything pushed so far; post it again after a new push.
/skip-bots — proceed without the bots that have not reported; anyone with write access may, and the report says who did.

This check passes when the policy is satisfied; the repository decides whether merging requires it.

thepastaclaw

Re-review — Final validation — Phase 1 + Phase 2

Verified the findings against f88edc5 and the lifecycle-only scope after b1442a7. Nine prior findings are fixed; the unchanged activation-migration concern belongs to stacked PR #4652 rather than blocking this PR. Forty-seven targeted Rust tests passed, but a temporary direct-Drive probe confirmed active-document history destruction, and existing WASM artifacts reproduced the empty batch getter and conflicting TypeScript declarations; the worktree is unchanged.

🔴 1 blocking | 🟡 4 suggestion(s)

5 finding(s) not shown inline (GitHub refused the PR diff as too large)

🔴 Blocking: Enforce the deleted-state precondition inside Drive's erase operation
packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs:153-164

The terminal erase branch does not read the lifecycle record or check that the current pointer is absent. Its lifecycle-record delete supplies a known leaf type, so it is not an existence check. A direct-Drive regression reproduced the consequence: create a document, delete and fully erase it, recreate the same ID, then erase it again without deleting it. Operation construction and application both succeed, but ordinary document reads subsequently fail because the active document's retained history was removed while its current pointer and indexes remain. The earlier erase leaves the per-type lifecycle container needed for this path. ABCI validation protects signed transitions, but the public Drive method and DocumentOperationType::EraseDocument bypass that validation. Require a valid deleted/erasing record and an absent current pointer before emitting revision deletions, and include those reads in estimation; identity authorization can remain in ABCI.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

🟡 Suggestion: Migrate the legacy WASM batch getter to the V1 accessor view
packages/wasm-dpp/src/document/state_transition/batch_transition/mod.rs:116-125

This getter still uses DocumentsBatchTransitionAccessorsV0::transitions_iter(), whose intentional format-2 view is empty. StateTransitionFactory.createFromBuffer() nevertheless accepts and wraps format-2 batches at protocol 15. Re-running the binding reproduction showed the same single-delete batch exposing one transition in both bindings at format 1, but one in wasm-dpp2 and zero in wasm-dpp at format 2. JavaScript inspection therefore reports no operations although serialization retains the actual batch. Keep the frozen accessor semantics, but migrate this adapter and its nested wrappers to the V1 accessor/shell, or explicitly reject unsupported batches at the decoding boundary.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

🟡 Suggestion: Avoid conflicting DocumentHistoryResult declarations
packages/wasm-sdk/src/queries/document.rs:194-197

The new custom interface merges with wasm-bindgen's exported DocumentHistoryResult class, but their properties disagree. The interface declares writable structural entries and an optional structural lifecycle, while the generated class declares readonly DocumentHistoryEntry[] and readonly DocumentHistoryLifecycle | undefined; the latter also exposes state as string rather than the interface's literal union. Checking the generated declarations with TypeScript and skipLibCheck disabled reproduces TS2687 and TS2717 errors for both properties. Consumers cannot check these declarations without suppressing library errors. Use the generated class as the canonical result type, or give the structural interface a distinct name and align the public return declarations.

_{source: gpt-6-astra (phase2-reviewer: ffi-engineer)}

🟡 Suggestion: Preserve contract-resolution costs in lifecycle delete conversion
packages/rs-drive/src/util/batch/drive_op_batch/document.rs:499-510

When DeleteDocumentWithLifecycle receives DataContractInfo::DataContractId, resolve() appends contract-fetch costs to drive_operations. This tail expression returns only the deletion operations and discards that accumulator, understating fees for callers using an unresolved contract ID. The normal transition converter supplies a resolved contract and is unaffected, but the new low-level operation explicitly accepts either form. Append the returned deletion operations to drive_operations and return the combined vector, as the adjacent EraseDocument branch does, and cover the contract-ID path.

_{source: gpt-6-astra (phase2-reviewer: rust-quality)}

🟡 Suggestion: Give the shared lifecycle cost estimator a versioned entry point
packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs:28-33

This helper defines tree-count and record-size assumptions used in fee estimation but ignores platform_version. It is shared by delete generation 1 and erase-estimation generation 0 rather than owned by either implementation generation. Their protocol-15 dispatch currently protects released protocols, so this is not an observed replay divergence; however, the book explicitly requires shared behavior reached from multiple generations to use a versioned helper. Add an optional estimation-method slot that is None in released tables and Some(0) in the protocol-15 table, with a fail-closed dispatcher, so later layout or sizing changes can preserve this calculation.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This intricate lifecycle change directly modifies consensus authorization and state validation in packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_erase_transition_action/state_v0/mod.rs and implements irreversible, chunked revision deletion in packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operati
Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (not used above high effort; tier asks max)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs:153-164: Enforce the deleted-state precondition inside Drive's erase operation
  The terminal erase branch does not read the lifecycle record or check that the current pointer is absent. Its lifecycle-record delete supplies a known leaf type, so it is not an existence check. A direct-Drive regression reproduced the consequence: create a document, delete and fully erase it, recreate the same ID, then erase it again without deleting it. Operation construction and application both succeed, but ordinary document reads subsequently fail because the active document's retained history was removed while its current pointer and indexes remain. The earlier erase leaves the per-type lifecycle container needed for this path. ABCI validation protects signed transitions, but the public Drive method and DocumentOperationType::EraseDocument bypass that validation. Require a valid deleted/erasing record and an absent current pointer before emitting revision deletions, and include those reads in estimation; identity authorization can remain in ABCI.

In `packages/wasm-dpp/src/document/state_transition/batch_transition/mod.rs`:
- [SUGGESTION] packages/wasm-dpp/src/document/state_transition/batch_transition/mod.rs:116-125: Migrate the legacy WASM batch getter to the V1 accessor view
  This getter still uses DocumentsBatchTransitionAccessorsV0::transitions_iter(), whose intentional format-2 view is empty. StateTransitionFactory.createFromBuffer() nevertheless accepts and wraps format-2 batches at protocol 15. Re-running the binding reproduction showed the same single-delete batch exposing one transition in both bindings at format 1, but one in wasm-dpp2 and zero in wasm-dpp at format 2. JavaScript inspection therefore reports no operations although serialization retains the actual batch. Keep the frozen accessor semantics, but migrate this adapter and its nested wrappers to the V1 accessor/shell, or explicitly reject unsupported batches at the decoding boundary.

In `packages/wasm-sdk/src/queries/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/queries/document.rs:194-197: Avoid conflicting DocumentHistoryResult declarations
  The new custom interface merges with wasm-bindgen's exported DocumentHistoryResult class, but their properties disagree. The interface declares writable structural entries and an optional structural lifecycle, while the generated class declares readonly DocumentHistoryEntry[] and readonly DocumentHistoryLifecycle | undefined; the latter also exposes state as string rather than the interface's literal union. Checking the generated declarations with TypeScript and skipLibCheck disabled reproduces TS2687 and TS2717 errors for both properties. Consumers cannot check these declarations without suppressing library errors. Use the generated class as the canonical result type, or give the structural interface a distinct name and align the public return declarations.

In `packages/rs-drive/src/util/batch/drive_op_batch/document.rs`:
- [SUGGESTION] packages/rs-drive/src/util/batch/drive_op_batch/document.rs:499-510: Preserve contract-resolution costs in lifecycle delete conversion
  When DeleteDocumentWithLifecycle receives DataContractInfo::DataContractId, resolve() appends contract-fetch costs to drive_operations. This tail expression returns only the deletion operations and discards that accumulator, understating fees for callers using an unresolved contract ID. The normal transition converter supplies a resolved contract and is unaffected, but the new low-level operation explicitly accepts either form. Append the returned deletion operations to drive_operations and return the combined vector, as the adjacent EraseDocument branch does, and cover the contract-ID path.

In `packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs:28-33: Give the shared lifecycle cost estimator a versioned entry point
  This helper defines tree-count and record-size assumptions used in fee estimation but ignores platform_version. It is shared by delete generation 1 and erase-estimation generation 0 rather than owned by either implementation generation. Their protocol-15 dispatch currently protects released protocols, so this is not an observed replay divergence; however, the book explicitly requires shared behavior reached from multiple generations to use a versioned helper. Add an optional estimation-method slot that is None in released tables and Some(0) in the protocol-15 table, with a fail-closed dispatcher, so later layout or sizing changes can preserve this calculation.

Out-of-scope follow-up suggestions (3)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Bound activation migration against growth beyond the measured inventory — history_migration_entries still uses insert_all() with no limit and materializes complete subtrees; callers also collect type-wide index references and revision copies during activation. A snapshot benchmark does not enforce an upper bound on later inventory. However, git diff b1442a7..HEAD shows no changes to this migration implementation, establishing that the concern belongs to the explicitly stacked storage PR #4652 rather than this lifecycle change.
- Follow-up: Track bounded migration work or an enforced activation-inventory bound with #4652, and retain an authentic-state resource rehearsal near activation.
Preserve the released history RPC under its existing wire version — Out of scope for this lifecycle PR. The incompatible request tags and relocated response document bytes already exist at b1442a7. The lifecycle-only proto diff appends DELETED/ERASING and four timestamp fields; it does not introduce the reported tag reuse. The lifecycle-only V0 handler changes adapt the expanded state enum and default the additional fields, rather than introducing the reported request rewrite.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
Harden generic try_to_u64 against unsafe JS Numbers past MAX_SAFE_INTEGER — Pre-existing generic conversion behavior outside this PR's exact history-selector boundary. The new selectors use ExactU64, and the offline binding checks reject all five unsafe Number positions before networking. Broadening the generic conversion fix would expand this PR's scope.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

…tocol 14 Close the coverage gaps the rework's review found. The kind's admission is now pinned end to end: a signed format 1 batch with an erase submitted through `process_raw_state_transitions` at protocol 14 is refused as `UnpaidConsensusError(UnsupportedVersionError)`, and at the dpp level the same batch decodes under protocol 14 through the in-version deserializer, names itself `DocumentsBatch([Erase])`, and is refused only by basic structure, which also restores the format 1 active-range assertion. The appended bincode discriminant is pinned: an erase and a delete over the same base encode identically except for one byte, 7 against 2, in batch formats 0 and 1, and decode back to an erase. The batched shell's `$transition` JSON case for an erase is added alongside. The platform-version pins are made literal again: the released loop asserts eleven batch-level slots by value for protocols 12 to 14, and the protocol 15 parity tuple compares twelve, so a change to both tables at once still fails. The batch action credit sums are checked with an erase alongside purchases. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

thepastaclaw

Re-review — Final validation — Phase 1 + Phase 2

The final wire refactor modifies shipped validation generations, violating the repository's explicit frozen-generation rule. Three additional in-scope suggestions remain: dropped contract-resolution costs, conflicting TypeScript declarations, and an unversioned shared cost estimator. All 15 prior findings were revalidated; 26 platform-version tests passed, and checking the available generated declarations reproduced the history-result typing errors.

🔴 1 blocking | 🟡 3 suggestion(s)

4 finding(s) not shown inline (GitHub refused the PR diff as too large)

🔴 Blocking: Keep erase processing out of shipped validation generations
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:154-161

This adds erase admission directly to basic-structure v0, which remains selected at both protocols 14 and 15. The PR also adds erase handling to ABCI's shipped transformer, advanced-structure, and state v0 implementations and Drive's execution-proof verifier v0. The protocol-14 rejection test demonstrates the new gate's behavior, but the book's frozen-generation rule explicitly prohibits adding internal version checks even when the new branch is unreachable for historical valid inputs. Restore the shipped implementations and place erase-aware behavior behind new version-dispatched generations selected only by protocol 15, with a decoding or admission boundary that excludes erase before entering released implementations.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)}

🟡 Suggestion: Preserve contract-resolution costs in lifecycle delete conversion
packages/rs-drive/src/util/batch/drive_op_batch/document.rs:499-510

The new DeleteDocumentWithLifecycle arm resolves its contract into drive_operations but returns only the deletion operations, discarding the resolution accumulator. DataContractInfo::DataContractId calls get_contract_with_fetch_info_and_add_to_operations, so this supported input can perform a charged contract fetch whose costs never reach the returned operation pipeline. ABCI's already-resolved DataContractFetchInfo avoids the loss, but does not make the public by-ID conversion correct. Append the deletion operations to drive_operations and return the combined vector, as the adjacent EraseDocument arm does; cover the by-ID path rather than only borrowed or already-resolved contracts.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering); gpt-6-astra (phase2-reviewer: general, architecture-layering, rust-quality)}

🟡 Suggestion: Avoid conflicting DocumentHistoryResult declarations
packages/wasm-sdk/src/queries/document.rs:194-197

This newly added interface shares its exported name with the class generated by #[wasm_bindgen(js_name = "DocumentHistoryResult")]. Their declarations cannot merge: the interface uses writable structural entries and an optional structural lifecycle, while the generated class exposes readonly DocumentHistoryEntry[] and DocumentHistoryLifecycle | undefined getters. The lifecycle state types also differ. Checking the available generated declarations reproduces TS2687 and TS2717 for these properties. Keep the generated class authoritative, or give a genuinely separate structural representation a different name; Rust/WASM compilation does not check this TypeScript declaration compatibility.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering); gpt-6-astra (phase2-reviewer: general, platform-versioning, rust-quality)}

🟡 Suggestion: Give the shared lifecycle cost estimator a versioned entry point
packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs:28-33

This shared helper defines fee-relevant layer counts and element sizes for both delete generation 1 and erase estimation generation 0, but ignores platform_version and has no owning implementation generation or dispatch slot. Its callers currently prevent pre-15 use, so this is not an immediate released-protocol fee divergence. However, changing the shared sizing later would also change estimates produced by those existing generations. Follow the book's shared-helper rule: add an optional method-version slot, leave it inactive in released tables, and dispatch protocol 15 to a frozen v0 implementation.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering, rust-quality); gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality)}

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This large, intricate change directly alters consensus validation and destructive storage operations in document_erase_transition_action/state_v0/mod.rs and erase_document_for_contract_operations/v0/mod.rs, requiring coordinated correctness of authorization, chunked erasure, retained-history invariants, and protocol-version gating.
Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (not used above high effort; tier asks max)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:154-161: Keep erase processing out of shipped validation generations
  This adds erase admission directly to basic-structure v0, which remains selected at both protocols 14 and 15. The PR also adds erase handling to ABCI's shipped transformer, advanced-structure, and state v0 implementations and Drive's execution-proof verifier v0. The protocol-14 rejection test demonstrates the new gate's behavior, but the book's frozen-generation rule explicitly prohibits adding internal version checks even when the new branch is unreachable for historical valid inputs. Restore the shipped implementations and place erase-aware behavior behind new version-dispatched generations selected only by protocol 15, with a decoding or admission boundary that excludes erase before entering released implementations.

In `packages/rs-drive/src/util/batch/drive_op_batch/document.rs`:
- [SUGGESTION] packages/rs-drive/src/util/batch/drive_op_batch/document.rs:499-510: Preserve contract-resolution costs in lifecycle delete conversion
  The new DeleteDocumentWithLifecycle arm resolves its contract into drive_operations but returns only the deletion operations, discarding the resolution accumulator. DataContractInfo::DataContractId calls get_contract_with_fetch_info_and_add_to_operations, so this supported input can perform a charged contract fetch whose costs never reach the returned operation pipeline. ABCI's already-resolved DataContractFetchInfo avoids the loss, but does not make the public by-ID conversion correct. Append the deletion operations to drive_operations and return the combined vector, as the adjacent EraseDocument arm does; cover the by-ID path rather than only borrowed or already-resolved contracts.

In `packages/wasm-sdk/src/queries/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/queries/document.rs:194-197: Avoid conflicting DocumentHistoryResult declarations
  This newly added interface shares its exported name with the class generated by #[wasm_bindgen(js_name = "DocumentHistoryResult")]. Their declarations cannot merge: the interface uses writable structural entries and an optional structural lifecycle, while the generated class exposes readonly DocumentHistoryEntry[] and DocumentHistoryLifecycle | undefined getters. The lifecycle state types also differ. Checking the available generated declarations reproduces TS2687 and TS2717 for these properties. Keep the generated class authoritative, or give a genuinely separate structural representation a different name; Rust/WASM compilation does not check this TypeScript declaration compatibility.

In `packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/estimation_costs/add_estimation_costs_for_document_lifecycle.rs:28-33: Give the shared lifecycle cost estimator a versioned entry point
  This shared helper defines fee-relevant layer counts and element sizes for both delete generation 1 and erase estimation generation 0, but ignores platform_version and has no owning implementation generation or dispatch slot. Its callers currently prevent pre-15 use, so this is not an immediate released-protocol fee divergence. However, changing the shared sizing later would also change estimates produced by those existing generations. Follow the book's shared-helper rule: add an optional method-version slot, leave it inactive in released tables, and dispatch protocol 15 to a frozen v0 implementation.

Out-of-scope follow-up suggestions (3)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Track activation migration sizing with the stacked storage PR — The migration still materializes unlimited subtree queries and completes synchronously in the activation transaction. However, both that implementation and protocol-15 activation already exist at the stated stacking boundary b1442a7; this lifecycle PR does not introduce or enlarge that migration work. The reported snapshot runtime is an operational measurement, not an enforced inventory bound.
- Follow-up: Keep final activation-inventory measurements and the authentic-state rehearsal with #4652's release tracking. Address bounded migration separately if the accepted activation envelope cannot be established.
Bound and paginate legacy history migration work — Out of scope for #4657: git diff b1442a7 HEAD shows no changes to the migration implementation or activation hook, and the boundary's PLATFORM_V15 already selects the same migration hook. The unbounded scan remains real but belongs to the stacked storage change and its explicitly deferred activation work; it is not a lifecycle regression.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
Give the replacement history RPC shape a new wire version — Out of scope for #4657: at b1442a7, the V0 request already uses tag 5 for prove and tag 7 for Cursor, and the response already uses entry tag 2 for revision. This PR's protobuf diff only appends lifecycle states and timestamp fields. The reported incompatible tag replacement predates the explicitly stated stacking boundary and should not be attributed to this lifecycle PR.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

… its estimator Three review findings on the keep-history lifecycle, none touching a released generation. The `DeleteDocumentWithLifecycle` operation resolved its contract into a local accumulator and returned only the delete's operations, so a caller naming the contract by id performed a charged fetch that never reached the operation pipeline. The delete's operations are now appended to the resolution's, as the erase operation already did; a test converts the same delete with a borrowed contract and with the contract id and pins that the by-id path adds exactly the fetch cost, first. The lifecycle-record estimator, shared by delete generation 1 and erase estimation generation 0, ran unversioned. It now sits behind `add_estimation_costs_for_lifecycle_record`, dispatched on an optional slot that is `None` in every released document method table and `Some(0)` at protocol 15, so a later change to its layer counts or record size cannot move the estimates of the generations that already rely on it. The slot joins the one-implementation-per-slot pin. wasm-sdk declared a structural `DocumentHistoryResult` interface with the same name as the class wasm-bindgen generates, and the two could not merge (TS2687 and TS2717 without `skipLibCheck`). The interface is gone; its documentation moved onto the generated getters, which the declarations carry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

shumkov · 2026-09-20T16:09:02Z

Dispositions for the PastaClaw round on e04c014777:

Blocking — keep erase processing out of shipped validation generations: declined, by design. The erase kind is carried the way every earlier kind was: document transfer (#1826), NFT purchase and price update (#1829), the token transitions (#2383) and the indexOnly delete (#4497, 9cf30789a5) were each appended to the shipped enums and handled by arms inside the existing v0 generations, with the batch action staying V0. Admission is gated per kind in basic structure v0 by a bounds slot that is None in every released table, exactly as #4497 did for document_index_only_delete_state_transition. At protocol 14 a node without the kind fails at decode and a node with it fails at the gate; both yield UnpaidConsensusError, so proposals agree. An earlier revision of this branch took the strict reading (wire format 2, action format 1, nine batch-level generations) and was replaced by a5a3d20450 for this reason; the PR body's "Wire and action shape" section records the choice.

Suggestion — contract-resolution costs in DeleteDocumentWithLifecycle: done in 1e8c9d3fa2. The delete's operations are appended to the resolution's, as the erase arm already did; a test converts the same delete with a borrowed contract and by contract id and pins that the by-id path emits the PreCalculatedFeeResult first and then exactly the delete's operations.

Suggestion — conflicting DocumentHistoryResult declarations: done in 1e8c9d3fa2. The structural interface is removed; the generated class is canonical, its getters carry the documentation, and state keeps the literal union through unchecked_return_type.

Suggestion — versioned entry point for the shared lifecycle estimator: done in 1e8c9d3fa2. add_estimation_costs_for_lifecycle_record now dispatches on DriveDocumentDeleteMethodVersions::add_estimation_costs_for_lifecycle_record, None in every released table and Some(0) at protocol 15, fail-closed; the slot joins the one-implementation-per-slot pin.

github-actions · 2026-09-20T22:24:23Z

@coderabbitai review

No review for 1e8c9d3f yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

thepastaclaw

⚠️ DEGRADED — Re-review — Preliminary review — Phase 1 blocker gate

⚠️ DEGRADED review. The primary review models were unavailable (gpt-6-astra unavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache), so this review ran on stand-in models: gpt-5.6-luna → muse-spark-1.3-contributor, gpt-5.6-sol → muse-spark-1.3-contributor, gpt-5.6-terra → muse-spark-1.3-contributor, gpt-6-astra → muse-spark-1.3-contributor. Both review phases and the independent verifiers still ran, but on weaker models, with Phase 1 capped at high effort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.

Lifecycle delete/erase is functionally complete and this push fixes the estimator versioning, contract-fetch billing, and WASM shape. Three blocking issues still hold at this head plus one test-coverage gap: shipped v0 validation was edited in place, Drive erase lacks a deleted-state precondition, and activation migration is unbounded.

Validated blockers were found by the Phase-1 review and confirmed by a fresh verifier. Phase 2 is deferred until a fresh same-head revalidation clears the blocker gate.

🔴 3 blocking | 🟡 1 suggestion(s)

4 finding(s) not shown inline (GitHub refused the PR diff as too large)

🔴 Blocking: Keep erase processing out of shipped validation generations
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:154-182

The erase admission gate still lives inside shipped validate_base_structure_v0, gated on document_erase_state_transition (None below 15). The book's frozen-generation rule explicitly forbids editing a shipped vN including adding a version-table check that is always false for old versions — replay safety must be structural, not reviewer-proved per diff. Byte-identity for historical inputs is necessary but not sufficient, and precedent #1826/#1829/#2383/#4497 predates that rule. Move erase handling to a new batch basic-structure generation (e.g. v1) selected only by PLATFORM_V15 tables, leaving v0 byte-identical.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering)}

🔴 Blocking: Enforce the deleted-state precondition inside Drive's erase operation
packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs:115-180

erase_document_for_contract_operations_v0 enumerates revision keys then on the terminal path deletes revisions, the lifecycle key, and the history subtree without ever fetching the lifecycle record (record fetch only happens on the non-terminal branch). Drive-abci state validation checks lifecycle on the consensus path, but any direct Drive caller or future action path bypasses it, so an Active keep-history document with a small history would lose revisions and its subtree. Fetch and match the lifecycle record first and return a deterministic State/consensus error unless Deleted or Erasing.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering)}

🔴 Blocking: Bound and paginate legacy history migration work
packages/rs-drive/src/drive/document/migration/mod.rs:459-499

history_migration_entries uses insert_all() with SizedQuery::new(query, None, None) and materializes the whole subtree inside the single activation transaction. Pre-14 keep-history types have no revision cap, so one large legal history forces every validator to read/deserialize/duplicate synchronously with no resume. Today's testnet inventory (~595 docs, ~2s) measures current size, not the bound an adversary or organic growth can create before mainnet activation. Page the scan with a finite limit or split activation into bounded resumable chunks.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, architecture-layering, security-auditor)}

🟡 Suggestion: Exercise proof verification when testing tampered lifecycle claims
packages/rs-drive/src/drive/document/lifecycle/tests.rs:1586-1639

The lifecycle tamper tests still corrupt proof bytes (push 0xff, flipping a byte mid-proof) and assert verification fails, which passes even if the verifier stopped checking lifecycle fields. The b30d969 harness proves deleted/erasing via maybe_from_proof but only flips one proof byte per stage rather than mutating each wire lifecycle field (state, remaining_revisions, four times) while leaving both proofs intact. Mutate each field independently through maybe_from_proof/verify and assert rejection so the test fails if field checks regress.

_{source: muse-spark-1.3-contributor (phase1-reviewer: architecture-layering)}

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); final verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: astra-gate-verifier, role: verifier)

Degraded mode: gpt-6-astra unavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache (detected by probe, since 2026-09-20T20:45:34Z); stand-ins gpt-5.6-luna → muse-spark-1.3-contributor, gpt-5.6-sol → muse-spark-1.3-contributor, gpt-5.6-terra → muse-spark-1.3-contributor, gpt-6-astra → muse-spark-1.3-contributor; Phase 1 effort capped at high
Triage: critical by muse-spark-1.3-contributor (standing in for gpt-6-astra) (effort low) — Large intricate change adds new consensus-validated erase/delete lifecycle and storage migration, e.g. document_erase_transition_action/state_v0 and drive/document/migration/mod.rs.
Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort high); agent phase1-reviewer
Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (zai below 15% reserve: 5h 100% left, weekly 13% left)
Fresh verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) — verifier; agent astra-gate-verifier
Phase 2 reviewers: not run (deferred by blocker gate)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:154-182: Keep erase processing out of shipped validation generations
  The erase admission gate still lives inside shipped `validate_base_structure_v0`, gated on `document_erase_state_transition` (None below 15). The book's frozen-generation rule explicitly forbids editing a shipped vN including adding a version-table check that is always false for old versions — replay safety must be structural, not reviewer-proved per diff. Byte-identity for historical inputs is necessary but not sufficient, and precedent #1826/#1829/#2383/#4497 predates that rule. Move erase handling to a new batch basic-structure generation (e.g. v1) selected only by PLATFORM_V15 tables, leaving v0 byte-identical.

In `packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs:115-180: Enforce the deleted-state precondition inside Drive's erase operation
  `erase_document_for_contract_operations_v0` enumerates revision keys then on the terminal path deletes revisions, the lifecycle key, and the history subtree without ever fetching the lifecycle record (record fetch only happens on the non-terminal branch). Drive-abci state validation checks lifecycle on the consensus path, but any direct Drive caller or future action path bypasses it, so an Active keep-history document with a small history would lose revisions and its subtree. Fetch and match the lifecycle record first and return a deterministic State/consensus error unless Deleted or Erasing.

In `packages/rs-drive/src/drive/document/migration/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/migration/mod.rs:459-499: Bound and paginate legacy history migration work
  (existing thread: https://github.com/dashpay/platform/pull/4657#discussion_r3995256896)
  `history_migration_entries` uses `insert_all()` with `SizedQuery::new(query, None, None)` and materializes the whole subtree inside the single activation transaction. Pre-14 keep-history types have no revision cap, so one large legal history forces every validator to read/deserialize/duplicate synchronously with no resume. Today's testnet inventory (~595 docs, ~2s) measures current size, not the bound an adversary or organic growth can create before mainnet activation. Page the scan with a finite limit or split activation into bounded resumable chunks.

In `packages/rs-drive/src/drive/document/lifecycle/tests.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/lifecycle/tests.rs:1586-1639: Exercise proof verification when testing tampered lifecycle claims
  (existing thread: https://github.com/dashpay/platform/pull/4657#discussion_r3995256902)
  The lifecycle tamper tests still corrupt proof bytes (`push 0xff`, flipping a byte mid-proof) and assert verification fails, which passes even if the verifier stopped checking lifecycle fields. The b30d969 harness proves deleted/erasing via `maybe_from_proof` but only flips one proof byte per stage rather than mutating each wire lifecycle field (state, remaining_revisions, four times) while leaving both proofs intact. Mutate each field independently through `maybe_from_proof`/`verify` and assert rejection so the test fails if field checks regress.

The erase operation read the lifecycle record only on a non-terminal chunk, where it needed the record's fields. A terminal chunk emitted its revision deletes without ever establishing that the document was deleted, so a caller of the public Drive method could strip an active document's retained history: a document that was never deleted, or one whose id was reused after an earlier erasure finished, both kept the per-type lifecycle tree the terminal path relied on. Transition validation refuses both before they reach Drive, but the method and the `EraseDocument` operation do not pass through it. The record is now read before anything is emitted, on every chunk. Its presence is what proves the document may lose revisions: it exists exactly while a document is deleted or erasing, and the insert generation of this protocol version refuses to create over a deleted id, so a record and a current pointer cannot coexist. A missing record, including a type that has no lifecycle tree at all, refuses the erase as invalid input. The admission estimate already priced this read on every chunk. A test erases a document that was never deleted and one recreated under a fully erased id; both are refused with every revision intact. The history query test pins that a proved response carries no lifecycle fields on the wire, so nothing a client reads there can stand in for what the proof derives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

shumkov · 2026-09-21T11:37:52Z

Dispositions for the PastaClaw round on 1e8c9d3fa2 (degraded run, per its own header):

Blocking — keep erase processing out of shipped validation generations: declined, as on the previous round. Same reasoning and the same precedent lineage; see the PR body's "Wire and action shape" and the earlier disposition comment.

Blocking — deleted-state precondition inside Drive's erase operation: done in 9c8532dee7. The lifecycle record is now read before anything is emitted, on every chunk, through the getter that tolerates a type with no lifecycle tree. Its presence is what proves the document may lose revisions: it exists exactly while a document is deleted or erasing, and the protocol 15 insert generation refuses to create over a deleted id, so a record and a current pointer cannot coexist; a current-pointer read would be redundant. A missing record refuses the erase as invalid input. The estimate already priced this read on every chunk. A test erases a document that was never deleted and one recreated under a fully erased id (your reproduction); both are refused with every revision intact.

Blocking — bound and paginate legacy history migration: out of scope here. The migration is #4652's and unchanged by this PR; the release decision on it is recorded there.

Suggestion — mutate each wire lifecycle field in the tamper tests: declined, with a pin added in 9c8532dee7. A proved getDocumentHistory response carries only the proof: result is a oneof of proof and history, so there are no lifecycle fields on the wire to mutate, and the verifier derives every lifecycle value from the proof. The Drive test already asserts each derived field per stage (state, remaining revisions, the four times), which fails if derivation regresses; the abci test now also asserts that the proved response carries no lifecycle claims.

Generation 2 repeated generation 1's pipeline with one option computed differently. It now performs its two lifecycle checks and calls generation 1 for everything else: `canBeErased` stays immutable, and an erasable type is refused before it can withdraw deletion, which is the one input on which generation 1's own option would have decided otherwise. Every other input reaches generation 1 with the options it always computed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…y query handler The document history query handler was introduced with the protocol 15 storage layout, so the generation that reports the deleted and erasing states with their times is that same handler, not a second one beside it. The copy is removed, its one distinct test moves to the handler's tests, and the `document_history_processing` slot and the query versions table that existed only to select the copy go with it. Before protocol 15 the lifecycle read yields only the active and absent states, so the handler answers identically there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…eletion The delete entry points that carry only a block time refused a keep-history document at the lifecycle generation, from inside their version dispatch, on the reasoning that they could not author the lifecycle entry. The entry needs the deletion time, which every entry point carries, and a deleter only to credit the record's bytes, which the fee-applying wrappers already omit on purpose. Nothing else of the block reaches the record, so the refusal protected nothing. The refusal is gone. Each dispatcher matches on the version alone, and the generation-1 body of every entry point delegates to the lifecycle-aware body with the block time it was given and no deleter. The test that pinned the refusal now pins that all three entry points record the deletion at the time they were given. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…instead of parallel methods The lifecycle delete needs the block and the deleter, so the delete entry points grew `_with_lifecycle` twins beside them, dispatched on the sibling's version slot, and the delete operation grew a `DeleteDocumentWithLifecycle` variant beside `DeleteDocument`. One method per directory means the signature changes instead: `delete_document_for_contract_operations`, its forced and post-drain forms, the named-type and by-id-named-type forms and the apply-and-add form now take `block_info: &BlockInfo` and `deleter_id: Option<Identifier>` in place of `block_time_ms`, and `DocumentOperationType::DeleteDocument` carries `deleter_id`. Generation 0 reads the time off the block and ignores the deleter, so released protocol versions behave as before; generation 1 records both. The generation-1 modules of `delete_document_for_contract`, its by-id form, the named-type form and the apply-and-add form existed only to forward the block and deleter to the twins. With the signature carrying them, they are the generation-0 bodies, so those modules and their version-table bumps are gone; the lifecycle decision stays in `delete_document_for_contract_operations` generation 1, where the behaviour lives. A delete converted from an operation naming its contract by id returns the contract fetch cost ahead of the delete's operations for every deleter, as the lifecycle variant already did. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…count State the rule that the delete lifecycle work first missed: when a new generation needs an input the callers do not pass yet, the method's signature grows and the callers change; a twin method, a twin operation variant or a wrapper that fabricates the value are the wrong shapes. The shipped generation keeps its own signature and receives the part of the input it always had. The delete entry points serve as the sample. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

github-actions · 2026-09-21T19:24:50Z

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-09-22T15:13:44.869Z

…izer already enforces The history selectors were wrapped in a type that refused a JavaScript number past `Number.MAX_SAFE_INTEGER` and took a `bigint` instead. The deserializer every query input already goes through does exactly that for a plain `u64`: a number is accepted only while it is a safe integer, a `bigint` at any `u64`, and anything else is an invalid type. The wrapper duplicated that rule behind a friendlier message, and its host test pinned `serde_json` rather than the deserializer the binding uses. The selectors are plain `u64` again. A wasm32 test builds the query object the way a caller does and pins the deserializer's rule: a `bigint` past the safe range arrives exact, and a number past it, a negative number or a fraction is refused. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ith platform serialization The lifecycle record of a keep-history document is read by proof verifiers as well as by Drive, so it belongs with the other stored types in rs-dpp rather than in Drive's own module. It moves to `dpp::document::lifecycle::DocumentLifecycleRecord` as a versioned enum over `DocumentLifecycleRecordV0` with the ordinary PlatformSerialize / PlatformDeserialize derive stack, and the hand-rolled fixed-width codec goes away. The fixed width existed so that an erase start could overwrite the record without changing its size. Nothing else in the codebase encodes that way, and a size change on overwrite is already handled: the batch apply merges the new bytes into the element's storage flags, so the deleter stays the beneficiary of what it paid for and the erase is charged for what its fields add. The estimator and the dry run now use the record's maximum encoded size, so the admission estimate stays an upper bound. Wire-visible only through stored bytes at protocol 15, which is unreleased. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

shumkov · 2026-09-21T21:58:56Z

/self-reviewed

…query The book had no chapter on keep-history document types: nothing on the protocol 15 storage layout (history tree, primary-key reference, lifecycle tree), on deleting and erasing such documents, on the four lifecycle states, or on what getDocumentHistory reports. This adds a Drive chapter covering the layout, the contract grammar (canBeDeleted on a keep-history type, canBeErased, the contested-index refusal), the delete and erase semantics with chunking, authorization-once and refund accounting, the query's selectors, lifecycle fields and two-proof envelope, the Rust and JavaScript client surfaces, and the versioning boundary. The evo-sdk state-transitions page now lists erase() and history() and links to it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

thepastaclaw

Re-review — Final validation — Phase 2 only (queue backlog)

Verified head b3e8fe2 and revalidated all 16 prior findings against the source. One blocking frozen-generation policy violation remains, together with a non-consensus Drive batching defect; the other prior findings are fixed, withdrawn, outdated, or outside this PR's scope. Independently ran 63 targeted Rust tests successfully with compiler warnings; WASM runtime and network suites were not rerun, and the tracked worktree remains unchanged.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

Triage: critical by gpt-6-astra (effort low) — This large, intricate diff changes consensus rules directly in packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_erase_transition_action/state_v0/mod.rs and consensus storage mutations in packages/rs-drive/src/drive/document/delete/erase_document_for_contract_operations/v0/mod.rs, introducing authorization, bounded irrevers
Phase 1 reviewers: not run (skipped for throughput: 27 PRs queued, above the 10 limit)
Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:154-162: Keep erase processing out of shipped validation generations
  The protocol-14 rejection argument is supported by the production-path regression: this is not a demonstrated proposal-acceptance divergence. The remaining conflict is with the explicit checked-in rule that shipped generations must not gain even dormant version-table checks. This diff adds the erase gate to DPP basic-structure v0 and executable erase branches to the existing ABCI transformer, advanced-structure and state generations and Drive execution-proof verifier. The passing `should_carry_the_erase_kind_without_a_new_batch_generation` test confirms that protocol 15 deliberately selects the same generations as protocol 14. Wire compatibility and method isolation are separate decisions: retaining the appended discriminant and existing batch/action encodings does not require retaining those method selections. Preserve the released executable implementations and select new erase-aware method generations only at protocol 15. The cited earlier transition additions establish precedent, but do not satisfy the current repository rule.

In `packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v1/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v1/mod.rs:339-347: Deduplicate lifecycle containers at the Drive batch boundary
  This existence check sees only the current document's local operations. `apply_drive_operations_v0` lowers sibling operations independently, and the DeleteDocument conversion passes no previous operations into the delete helper. Consequently, deleting two documents of the same type before its first lifecycle container exists emits duplicate inserts and rejects the entire batch. I reproduced the behavior with the passing `should_refuse_rather_than_half_apply_two_deletes_sharing_a_new_container` regression. The one-document transition cap prevents consensus from reaching this shape, and rejection is atomic, but the new delete capability still fails through Drive's public multi-operation API. Make lifecycle-container insertion aware of accumulated sibling operations, or resolve it during batch-wide lowering, so application and estimation account for the shared insertion once. Keep GroveDB's consistency checks intact.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Retain the activation-migration resource decision with the storage-v2 work — history_migration_entries still uses an unlimited range query and materializes the complete subtree. That implementation is unchanged between b1442a7 and the reviewed head, so it is not introduced or worsened by this PR. The reported testnet measurement supports that snapshot, but does not impose a bound on the inventory at activation.
- Follow-up: Keep authentic-state inventory measurements, validator resource limits, and activation acceptance criteria with #4652; pursue bounded migration separately if the supported activation inventory exceeds that envelope.

thepastaclaw · 2026-09-23T05:04:56Z

+                if let DocumentTransition::Erase(erase) = transition {
+                    let feature_version = match erase {
+                        DocumentEraseTransition::V0(_) => 0,
+                    };
+                    match &platform_version
+                        .dpp
+                        .state_transition_serialization_versions
+                        .document_erase_state_transition
+                    {


🔴 Blocking: Keep erase processing out of shipped validation generations

The protocol-14 rejection argument is supported by the production-path regression: this is not a demonstrated proposal-acceptance divergence. The remaining conflict is with the explicit checked-in rule that shipped generations must not gain even dormant version-table checks. This diff adds the erase gate to DPP basic-structure v0 and executable erase branches to the existing ABCI transformer, advanced-structure and state generations and Drive execution-proof verifier. The passing should_carry_the_erase_kind_without_a_new_batch_generation test confirms that protocol 15 deliberately selects the same generations as protocol 14. Wire compatibility and method isolation are separate decisions: retaining the appended discriminant and existing batch/action encodings does not require retaining those method selections. Preserve the released executable implementations and select new erase-aware method generations only at protocol 15. The cited earlier transition additions establish precedent, but do not satisfy the current repository rule.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality)}

thepastaclaw · 2026-09-23T05:04:56Z

+        let mut container_operations = Vec::new();
+        self.batch_insert_empty_tree_if_not_exists::<0>(
+            PathKey((std::mem::take(&mut document_type_path), lifecycle_tree_key)),
+            TreeType::NormalTree,
+            None,
+            tree_apply_type,
+            transaction,
+            &mut Some(batch_operations),
+            &mut container_operations,


🟡 Suggestion: Deduplicate lifecycle containers at the Drive batch boundary

This existence check sees only the current document's local operations. apply_drive_operations_v0 lowers sibling operations independently, and the DeleteDocument conversion passes no previous operations into the delete helper. Consequently, deleting two documents of the same type before its first lifecycle container exists emits duplicate inserts and rejects the entire batch. I reproduced the behavior with the passing should_refuse_rather_than_half_apply_two_deletes_sharing_a_new_container regression. The one-document transition cap prevents consensus from reaching this shape, and rejection is atomic, but the new delete capability still fails through Drive's public multi-operation API. Make lifecycle-container insertion aware of accumulated sibling operations, or resolve it during batch-wide lowering, so application and estimation account for the shared insertion once. Keep GroveDB's consistency checks intact.

_{source: gpt-6-astra (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

shumkov requested review from QuantumExplorer and lklimek as code owners September 10, 2026 22:55

shumkov changed the base branch from keep-history-storage-v2 to v4.2-dev September 10, 2026 23:05

shumkov closed this Sep 10, 2026

shumkov reopened this Sep 10, 2026

github-actions Bot added this to the v4.2.0 milestone Sep 10, 2026

shumkov force-pushed the keep-history-lifecycle branch from 492f4b5 to feea7d5 Compare September 11, 2026 00:47

thepastaclaw suggested changes Sep 11, 2026

View reviewed changes

Comment thread packages/wasm-sdk/src/state_transitions/document.rs Outdated

Comment thread packages/rs-drive/src/drive/document/history/mod.rs Outdated

shumkov force-pushed the keep-history-lifecycle branch from 6e27f84 to 25959ad Compare September 12, 2026 03:03

thepastaclaw suggested changes Sep 12, 2026

View reviewed changes

thepastaclaw reviewed Sep 12, 2026

View reviewed changes

Comment thread packages/wasm-sdk/src/queries/document.rs Outdated

thepastaclaw reviewed Sep 12, 2026

View reviewed changes

Comment thread packages/wasm-sdk/src/state_transitions/document.rs

Comment thread packages/rs-sdk/src/platform/documents/transitions/erase.rs

shumkov force-pushed the keep-history-lifecycle branch 4 times, most recently from 3d02cc9 to 4f6537b Compare September 13, 2026 12:35

thepastaclaw suggested changes Sep 13, 2026

View reviewed changes

QuantumExplorer changed the base branch from v4.2-dev to v4.3-dev September 15, 2026 19:14

github-actions Bot modified the milestones: v4.2.0, v4.3.0 Sep 15, 2026

shumkov and others added 7 commits September 19, 2026 16:01

shumkov requested review from HashEngineering, ZocoLini and romchornyi as code owners September 19, 2026 09:55

thepastaclaw suggested changes Sep 19, 2026

View reviewed changes

github-actions Bot added the bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. label Sep 20, 2026

github-actions Bot removed the bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. label Sep 20, 2026

github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Sep 20, 2026

thepastaclaw suggested changes Sep 20, 2026

View reviewed changes

github-actions Bot added the bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. label Sep 21, 2026

shumkov changed the base branch from v4.3-dev to keep-history-storage-v2 September 21, 2026 11:15

shumkov and others added 5 commits September 21, 2026 21:44

shumkov and others added 2 commits September 22, 2026 02:50

thepastaclaw suggested changes Sep 23, 2026

View reviewed changes

Conversation

shumkov commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Notes for reviewers

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

thepastaclaw commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 2 only (queue backlog)

Uh oh!

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 2 only (queue backlog)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 2 only (queue backlog)

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 2 only (queue backlog)

Uh oh!

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Preliminary review — Phase 1 blocker gate

Uh oh!

github-actions Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Hygiene

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Re-review — Final validation — Phase 1 + Phase 2

5 finding(s) not shown inline (GitHub refused the PR diff as too large)

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Re-review — Final validation — Phase 1 + Phase 2

4 finding(s) not shown inline (GitHub refused the PR diff as too large)

Uh oh!

shumkov commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 20, 2026

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

⚠️ DEGRADED — Re-review — Preliminary review — Phase 1 blocker gate

4 finding(s) not shown inline (GitHub refused the PR diff as too large)

Uh oh!

shumkov commented Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shumkov commented Sep 21, 2026

shumkov commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

thepastaclaw commented Sep 10, 2026 •

edited

Loading

codecov Bot commented Sep 11, 2026 •

edited

Loading

github-actions Bot commented Sep 19, 2026 •

edited

Loading

github-actions Bot commented Sep 21, 2026 •

edited

Loading